Skip to content

Make diagnostics parsing more idiomatic Effect#3857

Draft
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/idiomatic-effect-patterns-3619
Draft

Make diagnostics parsing more idiomatic Effect#3857
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/idiomatic-effect-patterns-3619

Conversation

@cursor

@cursor cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What Changed

  • Updated trace diagnostics NDJSON line parsing to use the shared decodeJsonResult(Schema.Record(...)) helper instead of JSON.parse.
  • Kept trace timestamp parsing and aggregate span bounds as Option values internally until returning the diagnostics result.
  • Updated process diagnostics so ProcessRow.pgid is represented as an Option and Windows PowerShell JSON parsing goes through Effect Schema decoding.
  • Added Windows process parsing coverage and updated process diagnostics tests to use assert from @effect/vitest.

Why

These changes address focused places where diagnostics code was already Effect-based but still used ad-hoc JSON parsing and nullable internal fields. Using Schema-backed JSON decoding and Option makes the parsing boundaries more testable and consistent with the rest of the Effect codebase while preserving the existing soft-fail diagnostics behavior.

UI Changes

Not applicable; this is a server diagnostics refactor with no UI changes.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Validation:

  • pnpm exec vp test run src/diagnostics/TraceDiagnostics.test.ts src/diagnostics/ProcessDiagnostics.test.ts src/diagnostics/ProcessResourceMonitor.test.ts (from apps/server) — 17 tests passed
  • pnpm exec vp check --fix — passed with existing warnings
  • pnpm exec vp run typecheck — passed
Open in Web View Automation 

Note

Refactor diagnostics parsing to use Effect Option types instead of nullables

  • Changes the pgid field on ProcessRow from number | null to Option.Option<number> across ProcessDiagnostics.ts, ProcessResourceMonitor.test.ts, and related tests.
  • Replaces JSON.parse with schema-based decoding (Schema, decodeJsonResult) in both parseWindowsProcessRows and aggregateTraceDiagnostics, so invalid JSON or malformed records now return empty results or are skipped rather than throwing.
  • Introduces typed WindowsProcessRecord schema with stricter validation: ProcessId must be a positive integer, ParentProcessId must be non-negative, and status defaults to 'Live' when absent.
  • Replaces nullable helper functions in TraceDiagnostics.ts with Option-returning variants throughout span aggregation logic.
  • Behavioral Change: Windows process rows with non-integer or out-of-range IDs are now silently dropped; non-JSON input to parseWindowsProcessRows now yields [] instead of throwing.

Macroscope summarized b5d4e92.

cursoragent and others added 3 commits July 10, 2026 16:06
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant